Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Windows] Fix titlebar not persisting when page is swapped #27192

Merged
merged 3 commits into from
Jan 28, 2025

Conversation

Foda
Copy link
Member

@Foda Foda commented Jan 16, 2025

Description of Change

  • Fix titlebar not being transferred when page is swapped
  • Fix titlebar leaking when replaced

Issues Fixed

Fixes #27170

Fix titlebar leaking events when replaced

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • src/Core/src/Platform/Windows/NavigationRootManager.cs: Evaluated as low risk
  • src/Core/src/Platform/Windows/WindowRootView.cs: Evaluated as low risk
Comments suppressed due to low confidence (1)

src/Controls/src/Core/Window/Window.cs:395

  • Ensure that there are test cases covering the cleanup and reattachment of the title bar when the page is swapped.
prevTitleBar.Cleanup();
@PureWeen PureWeen added this to the .NET 9 SR3.1 milestone Jan 17, 2025
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Playing with this it seems like the TitleBar still disappears?

tested in the sandbox

return new Window(new NavigationPage(new MainPage()))
{
	TitleBar = new TitleBar
	{
		BackgroundColor = Colors.Red,
		ForegroundColor = Colors.White,
		Subtitle = "Subtitle",
	}
};

Then in MainPage I swapped out the Page on Window

  <VerticalStackLayout>
    <Button Text="me" Clicked="Button_Clicked"></Button>
  </VerticalStackLayout>
	private void Button_Clicked(object sender, EventArgs e)
	{

		this.Window.Page = new MainPage();

    }

The TitleBar seems to reset when the page is swapped

@PureWeen
Copy link
Member

PureWeen commented Jan 23, 2025

Playing with this it seems like the TitleBar still disappears?

tested in the sandbox

Nevermind.....
I messed up the branch I was testing on and it didn't have your changes

Remove unneeded TB removal
Add leak test
@@ -120,7 +146,6 @@ public virtual void Disconnect()
}

SetToolbar(null);
SetTitleBar(null, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this call up into the WindowHandler.Disconnect code?

I think it's still useful to set the TitleBar to null if the handler itself is being disconnected

so we can add it here
image

after the disconnect call

@PureWeen PureWeen modified the milestones: .NET 9 SR3.1, .NET 9 SR4 Jan 28, 2025
@PureWeen PureWeen merged commit 3268462 into main Jan 28, 2025
101 of 104 checks passed
@PureWeen PureWeen deleted the foda/TBPageSwitch branch January 28, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Version 9.0.30] Window.TitleBar is set to null after changing Window.Page at runtime
2 participants